Skip to content

chore: fix full_bipartite_impl() #2124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: fix full_bipartite_impl() #2124

wants to merge 1 commit into from

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Sep 8, 2025 •
edited
Loading

👋 @szhorvat! @schochastics and I would need your help to fix full_bipartite_impl(), which might need to happen in the configuration files for Stimulus that are stored in the C library.

The full_bipartite_impl() function has those lines:

rigraph/R/aaa-auto.R

Lines 336 to 338 in 0ef90cc

if (igraph_opt("add.vertex.names") && is_named(graph)) {
names(res$types) <- vertex_attr(graph, "name")
}

That are wrong (no graph exists) but also, I think, useless?

@maelle
Copy link
Contributor Author

maelle commented Sep 8, 2025

Note that I think it is useless because other functions with those lines take graph as an input whereas this function creates a new graph from scratch. So where would names come from?

@maelle
Copy link
Contributor Author

maelle commented Sep 8, 2025

The same problem exists for biadjacency_impl() (reference to a not existing graph object in the body):

rigraph/R/aaa-auto.R

Lines 2579 to 2581 in 0ef90cc

if (igraph_opt("add.vertex.names") && is_named(graph)) {
names(res$types) <- vertex_attr(graph, "name", V(graph))
}

@maelle maelle requested a review from szhorvat October 9, 2025 10:33
@szhorvat
Copy link
Member

szhorvat commented Oct 9, 2025

@Antonov548 Do you remember what was the issue with the bipartite types parameter and autogeneration? I think the Stimulus core needed some update for the case when types was an output?

@Antonov548
Copy link
Contributor

Antonov548 commented Oct 10, 2025 •
edited
Loading

core needed some update

It's a hard to recall. But there was definitely some changes in Stimulus which were needed. But back in days we decided to make it with manual tweak.

@maelle maelle mentioned this pull request Oct 17, 2025
Open
95 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szhorvat szhorvat Awaiting requested review from szhorvat

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maelle @szhorvat @Antonov548